Validate submodule names before filesystem operations - #2202
Conversation
e1d07cc to
b88d913
Compare
Byron
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b88d913950
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
<!-- agent --> Submodule names read from .gitmodules can influence the separate Git directory path. Reject empty names, absolute or drive-qualified names, and parent components with either path separator. Validate before constructing module paths and before opening or mutating existing checkouts, so a repository initialized by an older vulnerable version cannot bypass the guard. Validate programmatic add and rename inputs before making changes as well. Advisory: GHSA-hmq2-w58f-27jc Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 <codex@openai.com>
b88d913 to
e4b8e7d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4b8e7d026
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This pull request hardens GitPython’s submodule handling by validating submodule names (as path-like identifiers) before performing filesystem operations, addressing a high-severity security advisory around path traversal via crafted submodule names.
Changes:
- Introduce centralized submodule name validation and apply it before key operations (module path construction, update/fetch, add/move/remove/rename, and module opening).
- Add regression tests covering invalid submodule names (parent components, rooted paths, drive-qualified paths) including scenarios involving separate git directories and
keep_going=Trueupdates.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
git/objects/submodule/base.py |
Adds _validated_name() and calls it from submodule operations to reject unsafe names before filesystem interactions. |
test/test_submodule.py |
Adds regression tests ensuring invalid submodule names are rejected and keep_going continues updating valid submodules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by Codex GPT-5.
Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Advisory
https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-hmq2-w58f-27jc
Advisory summary
This change validates submodule names before filesystem operations. Details are intentionally concise while the advisory remains a draft.
Changes
Git baseline
Matches the name-validation behavior introduced by Git commit
0383bbb901(submodule-config: verify submodule names as paths), with additional handling for Python path-join semantics.Validation
pytest -q test/test_submodule.pyon Python 3.14.6: 39 passed, 1 skipped, 1 xfailedgit/objects/submodule/base.py: passedNo Docker reproduction was used.